Move timez to logmatch. #67
Conversation
Remove side effect of config file creation if non-existent; this should be the responsibility of the caller if desired. Additionally, the user will not pick up new default timestamp formats once the config file is created, so this avoids that confusion.
There was a problem hiding this comment.
Pull request overview
This pull request moves the timez package from internal/pkg/timez to the external prequel-logmatch library (v0.0.19) and removes the side effect of automatic config file creation when the file doesn't exist. The changes ensure that users will always pick up new default timestamp formats from the library instead of having them frozen in a created config file.
Key changes:
- Moved timez package functionality to external prequel-logmatch library
- Removed automatic config file creation - now returns default config in memory when file doesn't exist
- Replaced
config.Marshal(),config.WriteDefaultConfig(), andconfig.LoadConfigFromBytes()withconfig.DefaultConfig()andconfig.ReadConfig()
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/pkg/timez/timez.go | Deleted - functionality moved to external library |
| internal/pkg/timez/timez_test.go | Deleted - tests moved to external library |
| internal/pkg/config/config.go | Removed Marshal/WriteDefaultConfig functions; added DefaultConfig and ReadConfig; uses timez.Defaults from external library |
| internal/pkg/config/config_test.go | Updated tests for new API; removed tests for removed functions; added comprehensive tests for new functions |
| pkg/eval/eval.go | Updated to use new config API with ReadConfig instead of LoadConfigFromBytes; imports external timez |
| internal/pkg/ux/ux.go | Updated import to use external timez package |
| internal/pkg/resolve/logfactory.go | Updated import to use external timez package |
| internal/pkg/cli/cli.go | Updated import to use external timez package |
| go.mod | Bumped prequel-logmatch dependency from v0.0.16 to v0.0.19 |
| go.sum | Updated checksums for new dependency version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… a Config object.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
internal/pkg/eval/eval.go:27
- The function dereferences the config pointer without checking if it's nil. Consider adding a nil check or documenting that the config parameter must not be nil to prevent potential nil pointer dereference panics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove side effect of config file creation if non-existent; this should be the responsibility of the caller if desired. Additionally, the user will not pick up new default timestamp formats once the config file is created, so this avoids that confusion.